Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Do not merge] dApp-friendly contract deployment #118

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

r-czajkowski
Copy link
Contributor

@r-czajkowski r-czajkowski commented Jul 28, 2022

Ref: threshold-network/token-dashboard#136

We need a dApp-friendly version of deployment allowing dashboard developers to build UI components without having to run geth/ganache/hardhat locally and deploying contracts locally.

This PR aims at presenting changes to smart contracts deployed to Sepolia that are used by the dApp development team for day-to-day work. Please do not merge this PR to main branch. Smart contract with the changes presented here are pushed to NPM registry with a tag dapp-development-sepolia.

Main changes:

  • sets the unstake period to 120s(2min) instead of 24h so we can test the unstake flow w/o waiting 24h,
  • adds a script that sets the minimum stake amount to 39999999999999999999999 (only for the Sepolia network).

Reduce the unstake period to 120s(2min) instead of 24h. Thanks to this
we can test the unstake flow w/o waiting 24h.
Run this script only for the goerli test network.
To protect the `*.sol` files. This is a protection to enforce the
never-merge rule for dApp development fine-tuned parameters.
@michalinacienciala
Copy link
Contributor

michalinacienciala commented Aug 2, 2022

Could you change this PR to draft and add a visible info in the description that those changes are not supposed to get merged to main?
Also, I was wondering if there's some way to additionally block the merging of this PR via GH settings. I mean something similar to protected branches functionality. From a quick googling I don't see such setting... Marking the PR as draft and explaining that the changes should not go to main will need to be enough then... [EDIT]: Oh, I see now that the CODEOWNERS file is there to protect us from unwanted merges. Good then 👍 But we could change PR to draft and add info to the description anyway.

@pdyraga pdyraga changed the title Dapp-friendly deployment [Do not merge] dApp-friendly contract deployment Aug 3, 2022
CODEOWNERS Outdated
@@ -0,0 +1 @@
*.sol @pdyraga @nkuba @lukasz-zimnoch
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's extract CODEOWNERS change to a separate PR and merge it to main.

The CODEOWNERS should cover all existing contracts from contracts/ directory

Let's add there all developers who worked on the Solidity code of the Threshold smart contracts: @vzotova @cygnusv @pdyraga

This will not only protect dApp-dev-only changes from being merged but will also protect from any accidental changes to an already audited code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


const { deployer } = await getNamedAccounts()

const minStakeAmount = "39999999999999999999999"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we want to set the minimum stake to 39999999999999999999999? With dApp team owning T token contract from dapp-development-goerli deployment, all devs should be able to mint any amount of T they want.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

39999999999999999999999 is the current min stake on mainnet that's why I picked this number but we could change to any number. We want to set the min stake to test the min stake validation in T dapp.

@r-czajkowski r-czajkowski marked this pull request as draft August 4, 2022 07:38
r-czajkowski and others added 5 commits August 11, 2022 16:36
Fix scritp that sets the min stake amount in the T token staking
contract. I've missed passing the `amount` argument to the
`setMinimumStakeAmount` function.
We are publishing packages with code from `dapp-development` branch under
versions that use `-dapp-dev-goerli.X` suffix. Our `package.json` on this branch
should reflect that. If we do not set it up, the CI job used to pubblish the
package will not be able to correctly bump up the version of the package.
Copy link

github-actions bot commented Nov 7, 2023

Solidity API documentation preview available in the artifacts of the https://github.com/threshold-network/solidity-contracts/actions/runs/6783566823 check.

We're switching deployment to the Sepolia testnet due to planned deprecation of
the Goerli testnet. In a previous commit we've updated the `dapp-development`
branch with the recent changes from `main` (among them were the changes adding
Sepolia to the list of supported networks). Now we're updating the
`dapp-development` branch to create `dapp-dev-sepolia` packages.
Copy link

github-actions bot commented Nov 7, 2023

Solidity API documentation preview available in the artifacts of the https://github.com/threshold-network/solidity-contracts/actions/runs/6783635251 check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants